fix(l10n): stop re-pushing a per-app language override the user cleared - #810
Open
EugeneSusla wants to merge 3 commits into
Open
fix(l10n): stop re-pushing a per-app language override the user cleared#810EugeneSusla wants to merge 3 commits into
EugeneSusla wants to merge 3 commits into
Conversation
The launch-time reconciliation added with the per-app language picker (simonoppowa#672) pushed the saved in-app language to the OS whenever the OS reported no override. That push was meant as a one-time migration for installs predating the wiring, but nothing recorded that it had happened, so it ran on every launch — and "no override" is also exactly what the OS reports after the user picks System default in Android's per-app picker. Their explicit clear was silently reinstated on the next cold start, and the only way off the override was the in-app picker, with nothing suggesting so. Record the migration in config (localeSyncSeeded, Hive index 37) and decide the ambiguous cases on the raw system tag instead of the folded language code: - Supported override: system wins, saved if different, seeded recorded (both sides agree; the migration must never fire again). - Override we do not ship (e.g. ja): ignored on both sides. Previously the saved code was pushed over it, overwriting an explicit OS-level choice the app merely cannot render. - No override, not yet seeded: the one-time migration push, now recorded as seeded. - No override, seeded: the user cleared it — follow the system and clear the in-app override too. The runtime path (didChangeLocales -> _adoptSystemLocale) gets the same raw-tag treatment, so clearing the override while the app is alive now switches the app back to the system language instead of doing nothing.
Review follow-ups on the seeded flag's semantics: - localeSyncSeeded now records exactly one fact: the OS was SEEN holding an override. Recording it on the migration attempt turned every platform that can never hold one (iOS, desktop, Android < 13) into a "user who cleared it" on the second launch, silently resetting their in-app language. The migration branch now repeats harmlessly there; on Android 13+ the pushed value is observed on the next read and seeding completes then. - An unshipped override tag also records seeded — the OS demonstrably holds a value — so switching ja to System default later reads as the clear it is instead of re-triggering the migration push. - AppLocaleService.getApplicationLocale now reports read failures distinctly instead of folding PlatformException into "no override"; reconcileAppLocale decides nothing on a failed read, so a transient channel error can no longer clear a saved language. MissingPluginException stays a clean "no override": on those platforms the absence is permanent and true. - _adoptSystemLocale now runs reconcileAppLocale itself instead of hand-rolling a second copy of the decision table, so the launch and runtime paths cannot disagree about the same OS state; only the LocaleProvider update stays local. This also gives the runtime path the seeded guard it was missing — a system-language change on an override-less platform no longer wipes the in-app choice.
Recording seeded only on a later, separate observation left a window — from the migration push until the next read — in which a user's clear in Android's picker still read as never-seeded and was re-pushed. The window never closed at all when the pushed language matched the device language, since no locale change meant no didChangeLocales and no read. Read the tag straight back inside the migration branch instead: on a platform that holds per-app overrides the pushed value echoes back immediately and seeded is recorded before the call returns, so any later clear is honoured; on an override-less platform the read-back stays empty and the branch keeps harmlessly repeating. This also stops the migration push re-running on every launch and locale callback on capable devices — it now runs once. _adoptSystemLocale is serialized against itself: reconciling can change the locale (the push), which re-fires didChangeLocales mid-run, and two interleaved runs would double-apply one config snapshot. A run arriving early queues a replay rather than being dropped, so a clear landing mid-run is still picked up. Tests cover the push-echo seeding, the override-less repeat (pushes asserted), and the failed-read guard.
Owner
Hold this out of 2.2.0 — there is a Hive field-index collision in itFlagged by a release-readiness audit before #988, not by review of the fix itself. The change carries a Hive field-index collision, which means resolving it also means regenerating The underlying fix — not re-pushing a per-app language override the user cleared — is real and worth landing. This is about when, not whether. Recommend: resolve the index collision on its own, with its own regeneration and review, after 2.2.0 is out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #672. The launch-time reconciliation pushes the saved in-app language to the OS whenever the OS reports no override. That push was meant as a one-time migration for installs that predate the wiring, but nothing records that it happened — and "no override" is also exactly what the OS reports after the user picks System default in Android's per-app picker. Their clear is silently re-pushed on the next cold start, so the only way off the override is the in-app picker, with nothing suggesting so. Two neighbouring cases share the flaw:
ja) folds to the same null, so the saved code was pushed over the user's OS-level choice — overwriting it rather than ignoring it._adoptSystemLocaleearly-returned on null, the inert-picker failure in the clearing direction.The fix
Config gains
localeSyncSeeded(Hive index 37), recording exactly one fact: the OS has been seen holding an override.reconcileAppLocaledecides the ambiguous cases on the raw system tag instead of the folded language code:AppLocaleService.getApplicationLocalenow reportsreadFaileddistinctly instead of foldingPlatformExceptioninto "no override" (MissingPluginExceptionstays a clean "no override": on iOS/desktop the absence is permanent and true)._adoptSystemLocalenow runs the samereconcileAppLocaleinstead of hand-rolling a second copy of the decision table, which gives the runtime path the seeded guard and the failed-read guard for free — and fixes the clearing direction while the app is alive. It is also serialized against itself: the migration push re-firesdidChangeLocalesmid-run, and two interleaved runs would double-apply one config snapshot; an early arrival queues a replay rather than being dropped.Verification
flutter analyzeclean; full suite green (1093 tests).